Skip to content

fix: prevent SubClassFinder availability crash#8457

Draft
philipphofmann wants to merge 21 commits into
mainfrom
fix/subclassfinder-availability-crash
Draft

fix: prevent SubClassFinder availability crash#8457
philipphofmann wants to merge 21 commits into
mainfrom
fix/subclassfinder-availability-crash

Conversation

@philipphofmann

@philipphofmann philipphofmann commented Jul 17, 2026

Copy link
Copy Markdown
Member

📜 Description

Discover UIViewController subclasses without realizing classes. Instead of objc_copyClassNamesForImage + NSClassFromString, SentryDefaultObjCRuntimeWrapper.classes(forImage:) reads the image's __objc_classlist Mach-O section to get class pointers (dyld-bound, not realized), then reuses the existing class_getSuperclass subclass walk. The confirmed view-controller class pointers are handed straight to the swizzle block on the main thread — NSClassFromString is no longer used (it realizes the class, and could resolve a same-named class from a different image). The swizzling logic itself is unchanged; only class discovery changed.

Neither the superclass walk (class_getSuperclass) nor class_getName sends a message to the class, so +initialize is never triggered on the background thread — important because UIViewControllers assume they run on the main thread.

Guarded with MH_MAGIC_64 before rebinding to mach_header_64 (also skips FAT headers), gated to 64-bit archs, and documented as off-main-thread only (the _dyld_* calls take the dyld loader lock).

💡 Motivation and Context

Fixes #8152. On SDK start, the old code called NSClassFromString on every class in the app image, which realizes it. Realizing a Swift class whose metadata references an @available-gated newer-framework type crashes with EXC_BAD_ACCESS on OS versions below that framework's availability (real devices only). Real-world crashers aren't view controllers — SwiftUI gesture Coordinators, RoomPlan/ActivityKit wrappers — so the SDK must avoid realizing unrelated classes. Underlying Swift/ObjC runtime bug: swiftlang/swift#72657.

💚 How did you test it?

  • Unit (9 tests): a differential test asserts the new __objc_classlist enumeration matches objc_copyClassNamesForImage across every loaded image; a section-parsing test covers null entries; a regression test drives the real wrapper through the finder against the test bundle; and testGettingSubclasses_DoesNotCallInitializer guards that discovery never triggers +initialize.
  • On-device arm64e: built the iOS-Swift sample ARCHS=arm64e and ran it on a physical iPhone 12 (A14, iOS 26.5) — 44 classrefs read, 31 view controllers swizzled, zero EXC_BAD_ACCESS. Confirms the raw classref read is safe on PAC-signed __objc_classlist entries (dyld applies chained fixups in place at load time). Repro steps in this comment.

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.
  • If I added a new public API, I also added it to the SentryObjC wrapper.

getsectiondata with mach_header_64 doesn't compile on 32-bit watchOS
device slices (arm64_32, armv7k). Match the only caller,
SentrySubClassFinder, which is iOS/tvOS/visionOS only.
Comment thread Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift
Comment thread Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift Outdated
Comment thread Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift
Comment thread Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift
Comment thread Sources/Swift/Helper/SentryDefaultObjCRuntimeWrapper.swift

let count = Int(size) / MemoryLayout<UnsafeRawPointer>.size
return section.withMemoryRebound(to: UnsafeRawPointer.self, capacity: count) { classes in
(0..<count).map { unsafeBitCast(classes[$0], to: AnyClass.self) }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: I don't expect any issue here, but just double check this works on arm64e due to pointer authentication

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-checked. This is safe on arm64e:

  • The unsafeBitCast only reinterprets the pointer's type (UnsafeRawPointerAnyClass); it doesn't strip or re-sign anything. These are the exact classref pointers the ObjC runtime itself stores in __objc_classlist and hands to the loader, so they're already correctly formed for the runtime.
  • We never do hand-rolled pointer authentication/stripping. Every subsequent use goes through the runtime (class_getSuperclass, class_getName), which authenticates internally.
  • App Store binaries ship plain arm64, so arm64e mostly matters for the dyld shared cache / system frameworks, which we walk the same way.

Definitive confirmation needs a real arm64e device run (CI sims are arm64/x86_64); tracked as device-only follow-up in the PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated safe on arm64e. Reading __objc_classlist raw and unsafeBitCast-ing to AnyClass works on arm64e because dyld applies chained fixups in place at load time — by the time we read the in-memory section the entries are already runtime-correct pointers, so class_getSuperclass/class_getName accept them directly with no ptrauth strip. Confirmed on a physical arm64e device: 44 classrefs read, 31 view controllers swizzled, zero EXC_BAD_ACCESS.

How this was verified with Claude Code (reproducible)

This investigation was done with Claude Code. To re-run it, plug in an A12+ physical device (iPhone XS or newer — arm64e requires A12), open this branch, and paste the following into Claude Code:

Build the iOS-Swift sample as arm64e and run it on my connected physical device to check whether SentryDefaultObjCRuntimeWrapper.classes(forImage:) is safe when it reads PAC-signed __objc_classlist entries. Steps:

  1. Get the device UDID: xcrun devicectl list devices (find the connected physical one).
  2. Build for that device forcing the arm64e slice, using the project's own signing (do NOT override CODE_SIGN_STYLE/DEVELOPMENT_TEAM — the sample already has manual signing configured):
    xcodebuild -project Samples/iOS-Swift/iOS-Swift.xcodeproj -scheme iOS-Swift \
      -configuration Debug -destination 'id=<UDID>' -derivedDataPath /tmp/arm64e-build \
      ARCHS=arm64e ONLY_ACTIVE_ARCH=NO build
    
  3. Confirm the built slices are actually arm64e:
    lipo -archs /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app/iOS-Swift
    lipo -archs /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app/iOS-Swift.debug.dylib
    
    (Debug builds put the Swift classes — and thus __objc_classlist — in iOS-Swift.debug.dylib, so that one matters most.)
  4. Install and launch with the console attached:
    xcrun devicectl device install app --device <UDID> /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app
    xcrun devicectl device process launch --console --device <UDID> io.sentry.sample.iOS-Swift
    
  5. In the console output, verify: (a) SentrySubClassFinder logs Found N number of classes in image: .../iOS-Swift.debug.dylib, (b) it logs The following UIViewControllers ... will generate automatic transactions: <list of iOS_Swift.* VCs>, and (c) there is no EXC_BAD_ACCESS/SIGSEGV/crash and the process stays alive.

If it reads the classes, names them, and swizzles the app's view controllers without crashing, the raw arm64e classref read is safe. If it crashes in class_getSuperclass/class_getName, a ptrauth-aware read would be required.

Result on an iPhone 12 (A14, iOS 26.5): app + iOS-Swift.debug.dylib + Sentry.framework all arm64e; SentrySubClassFinder read 44 classrefs from the arm64e __objc_classlist, and SentryUIViewControllerSwizzling swizzled 31 UIViewController subclasses (BenchmarkingViewController, CoreDataViewController, nested/mangled ones like _TtCC9iOS_Swift18PageViewController17RedViewController, …). No crash across two launches; process stayed alive at a live UI.

Why this is the right test: the PAC signing of __objc_classlist entries depends on the slice the image was built as, not just the CPU — an arm64 app on an A12+ device has no signed classrefs. Forcing ARCHS=arm64e on the app (and its .debug.dylib) is what actually exercises signed classrefs. CI and the simulator run arm64/x86_64 where ptrauth is a no-op, which is why this needs a device.

The section read binds to mach_header_64, so guard the conditional
with arch(arm64) || arch(x86_64) to exclude 32-bit watchOS device
slices (arm64_32, armv7k). Covers all slices iOS/tvOS/visionOS ship.
Guard classes(forImage:) with MH_MAGIC_64 before rebinding the header
to mach_header_64, which also safely skips a FAT header instead of
misreading it. Document that _dyld_get_image_header/_dyld_get_image_name
take the dyld loader read lock, so the method must run off the main
thread (its only caller already does).

Drop the swizzleClassNameExcludes doc paragraph, remove the sample
RoomPlan repro scaffolding, and add a regression test driving the real
runtime wrapper against the test bundle.
@philipphofmann

Copy link
Copy Markdown
Member Author

Pushed b4755ae7c addressing the review feedback:

  • MH_MAGIC_64 guard (h) before rebinding to mach_header_64 — also safely skips a FAT header rather than misreading it (no FAT parsing needed; _dyld_get_image_header only returns thin in-memory slices).
  • arm64e (m) — reasoned through why the unsafeBitCast is safe (no hand-stripping; runtime authenticates); device run tracked as follow-up.
  • arch guard (m) — done earlier in 62b002c8d.
  • dyld loader lock (self-found while checking the above): _dyld_get_image_header/_dyld_get_image_name take the loader read lock, so classes(forImage:) now documents it must run off the main thread (the sole caller already does).
  • Removed the swizzleClassNameExcludes doc paragraph, dropped the sample RoomPlan repro scaffolding, and added a real-wrapper regression test.

Changelog now references #8457. Leaving as a draft for a final look before marking ready.

Lead with the off-main-thread requirement, drop caller references from
the doc comments, and compare the image name with strcmp instead of
round-tripping the C string through String.
`magic` is the first field of both `mach_header` and `mach_header_64`,
so read it on the original pointer and return [] directly instead of
funneling nil through the withMemoryRebound closure.
Explain we don't re-read _dyld_image_count per iteration like the
CxaThrowSwapper, since we search for one already-loaded image.
SwiftUI was only referenced in comments, never in code.
The defensive early returns (missing header, non-mach_header_64,
absent __objc_classlist, image not loaded) were silent, so a class
list coming back empty was hard to triage. Add short log lines.
Follow the test<Function>_when<Condition>_should<Expected> naming
convention and the arrange-act-assert layout.
classes(forImage:) confirmed safe on a physical arm64e iPhone 12: 44
classrefs read, 31 view controllers swizzled, no EXC_BAD_ACCESS.
@philipphofmann
philipphofmann force-pushed the fix/subclassfinder-availability-crash branch from ae2d6da to b83614b Compare July 21, 2026 08:08
The old comment was imprecise for the current flow. Explain that the
background walk uses only class_getSuperclass and class_getName, which
never message the class and so never trigger +initialize, and that we
hand names to the main thread where messaging the class is safe.
Rebind the __objc_classlist section to AnyClass? — the honest
Swift type of its Class _Nullable entries — and compactMap out
nulls, instead of reading non-optional UnsafeRawPointer and
unsafeBitCast-ing each entry to AnyClass. A null entry in a
malformed section is now skipped instead of producing an
invalid non-optional AnyClass (undefined behavior).

Extract the section parsing into an internal static helper so
tests can exercise the null-entry case, which a real
dyld-loaded section can't be made to contain.
Feed the new classes(inSection:size:) helper a crafted buffer
shaped like getsectiondata's return value with a null slot in
the middle, and assert the null is skipped. A real dyld-loaded
__objc_classlist section never contains nulls, so this edge
case is unreachable through classes(forImage:).
Store AnyClass instead of class names in SentrySubClassFinder and
call the swizzle block with the stored pointer. This drops the
NSClassFromString round-trip on the main thread, which realizes the
class and could re-trigger the availability-gated realization crash
this branch fixes (GH-8152, swiftlang/swift#72657). It also swizzles
exactly the class found in the target image instead of whatever a
name lookup resolves for duplicate class names.
…ailability-crash

# Conflicts:
#	Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift
#	Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift
Reflect the direct class-pointer handoff (no NSClassFromString), the
merged origin/main (#8494 early return), the extracted
classes(inSection:) helper, and the current test names.
Unremapped __objc_classlist entries (Finding 2) still reach the
swizzler; flag it in the wrapper comment and handoff. Add a filter
test that documents it does not close the finding. No behavior change.

@noahsmartin noahsmartin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall approach approach looks good to me, I would just advise testing on all platforms (watchOS, tvOS, etc...) before shipping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SentrySubClassFinder crashes on OS-gated UIViewController subclasses

4 participants